Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

deps: V8: cherry-pick e3d7f8a #29105

Closed
wants to merge 1 commit into from
Closed

deps: V8: cherry-pick e3d7f8a #29105

wants to merge 1 commit into from

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Aug 13, 2019

Python 2: dict.keys() is a list.
Python 3: dist.keys() is an iterator.
These changes support both Python 2 and Python 3.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. v8 engine Issues and PRs related to the V8 dependency. labels Aug 13, 2019
@cclauss cclauss added the python PRs and issues that require attention from people who are familiar with Python. label Aug 13, 2019
@cjihrig
Copy link
Contributor

cjihrig commented Aug 13, 2019

I think these changes should be upstreamed to V8 first.

@cclauss
Copy link
Contributor Author

cclauss commented Aug 13, 2019

@cjihrig Sorry but I respectfully disagree. These are blocking changes to our progress. The v8 process just takes too long (proof: #24512) and we only have 141 days until Py2 EOL. The folks responsible for v8 do not want to see this transition succeed. If someone else has the patience to upstream these (minuscule) changes in parallel or after, they are invited to do so.

@targos
Copy link
Member

targos commented Aug 13, 2019

I opened https://chromium-review.googlesource.com/c/v8/v8/+/1751331 upstream.

@cclauss cclauss mentioned this pull request Aug 13, 2019
Closed
4 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/24966/

@sam-github
Copy link
Contributor

Floating patches on v8 increase maintenance burden. Lets give some time for @targos' upstream PR to land (or not land :-( if that is the unfortunate case) before floating a patch.

@cclauss cclauss requested review from sam-github and refack August 13, 2019 16:34
MattIPv4
MattIPv4 reviewed Aug 14, 2019
View reviewed changes
Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we're updating this file, why not also drop the semi-colons?

@MattIPv4 MattIPv4 mentioned this pull request Aug 14, 2019
Closed
1 task
@cclauss @targos
deps: V8: cherry-pick e3d7f8a
ce33ffb 
Original commit message:

    [build] update gen-postmortem-metadata for Python 3

    This change makes the code compatible with both Python 2 and Python 3.

    Change-Id: I99d68af9c3163607c3a2fdbafac339a98b7471e4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751331
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63207}

Refs: v8/v8@e3d7f8a
@targos targos force-pushed the Py3_gen-postmortem-metadata.py branch from 06ff12a to ce33ffb Compare August 14, 2019 12:58
@targos
Copy link
Member

targos commented Aug 14, 2019

The upstream commit landed. I updated this PR to cherry-pick it.

@cclauss cclauss mentioned this pull request Aug 14, 2019
Closed
63 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25007/

@Trott Trott changed the title v8: py3 gen-postmortem-metadata.py deps: V8: cherry-pick e3d7f8a Aug 16, 2019
@Trott
Copy link
Member

Trott commented Aug 16, 2019

Landed in 1173199

@Trott Trott closed this Aug 16, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Aug 16, 2019
@cclauss @Trott
deps: V8: cherry-pick e3d7f8a
1173199 
Original commit message:

    [build] update gen-postmortem-metadata for Python 3

    This change makes the code compatible with both Python 2 and Python 3.

    Change-Id: I99d68af9c3163607c3a2fdbafac339a98b7471e4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751331
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63207}

Refs: v8/v8@e3d7f8a

PR-URL: nodejs#29105
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@cclauss cclauss deleted the Py3_gen-postmortem-metadata.py branch August 16, 2019 05:22
targos pushed a commit to targos/node that referenced this pull request Aug 19, 2019
@cclauss @targos
deps: V8: cherry-pick e3d7f8a
02132d0 
Original commit message:

    [build] update gen-postmortem-metadata for Python 3

    This change makes the code compatible with both Python 2 and Python 3.

    Change-Id: I99d68af9c3163607c3a2fdbafac339a98b7471e4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751331
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63207}

Refs: v8/v8@e3d7f8a

PR-URL: nodejs#29105
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Aug 19, 2019
@cclauss @targos
deps: V8: cherry-pick e3d7f8a
a17d398 
Original commit message:

    [build] update gen-postmortem-metadata for Python 3

    This change makes the code compatible with both Python 2 and Python 3.

    Change-Id: I99d68af9c3163607c3a2fdbafac339a98b7471e4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751331
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63207}

Refs: v8/v8@e3d7f8a

PR-URL: #29105
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos mentioned this pull request Aug 19, 2019
Merged
This was referenced Aug 20, 2019
Closed
Closed
Closed
Closed
Closed
Closed
targos pushed a commit to targos/node that referenced this pull request Sep 17, 2019
@cclauss @targos
deps: V8: cherry-pick e3d7f8a
7759de6 
Original commit message:

    [build] update gen-postmortem-metadata for Python 3

    This change makes the code compatible with both Python 2 and Python 3.

    Change-Id: I99d68af9c3163607c3a2fdbafac339a98b7471e4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751331
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63207}

Refs: v8/v8@e3d7f8a

PR-URL: nodejs#29105
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Sep 19, 2019
@cclauss @targos
deps: V8: cherry-pick e3d7f8a
60efc5f 
Original commit message:

    [build] update gen-postmortem-metadata for Python 3

    This change makes the code compatible with both Python 2 and Python 3.

    Change-Id: I99d68af9c3163607c3a2fdbafac339a98b7471e4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751331
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63207}

Refs: v8/v8@e3d7f8a

Backport-PR-URL: #29241
PR-URL: #29105
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
Merged
This was referenced Sep 25, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MattIPv4 MattIPv4 MattIPv4 left review comments

@cjihrig cjihrig cjihrig approved these changes

@Trott Trott Trott approved these changes

@sam-github sam-github sam-github approved these changes

@refack refack Awaiting requested review from refack

Assignees
No one assigned
Labels
post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. python PRs and issues that require attention from people who are familiar with Python. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cclauss @cjihrig @targos @nodejs-github-bot @sam-github @Trott @MattIPv4